feat(storage): Add full object checksum validation for appendable uploads#16110
feat(storage): Add full object checksum validation for appendable uploads#16110v-pratap wants to merge 15 commits into
Conversation
product-auto-label
Bot
added
the
api: storage
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements dynamic CRC32C validation for appendable object uploads by updating the connection and writer logic to initialize hash functions from RPC response metadata. It also adds a stateful constructor to the CRC32C hash function. Feedback highlights a critical regression where a large suite of existing integration tests was deleted and replaced with experimental code. Additional suggestions include adding defensive checks for CRC32C field presence, utilizing existing library thread management instead of a custom thread pool in tests, and using environment variables instead of hardcoded strings for test configuration.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16110 +/- ##
==========================================
- Coverage 92.71% 92.71% -0.01%
==========================================
Files 2353 2353
Lines 218986 219098 +112
==========================================
+ Hits 203031 203129 +98
- Misses 15955 15969 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
v-pratap
force-pushed
the
fastbyte-full-object-checksum
branch
from
18389a0 to
58d4b1d
Compare
| // Regular resume succeeded, object not finalized. Continue writing. | ||
| auto persisted_offset = absl::get<std::int64_t>(state); | ||
|
|
||
| auto impl_concrete = dynamic_cast<AsyncWriterConnectionImpl*>(impl_.get()); |
There was a problem hiding this comment.
Thanks for adding a fix for the edge case.
Can we avoid dynamic_cast and instead add a checksum tracking API in the virtual interface base class (writer_connection.h)? We could use a default no-op implementation and override under writer_connection_impl.h.
Ref: https://google.github.io/styleguide/cppguide.html#Run-Time_Type_Information__RTTI_
3 participants
No description provided.